New lint: decimal_bit_mask
#15215
New lint: decimal_bit_mask
#15215
Conversation
|
Lintcheck changes for 12ab9f6
This comment will be updated if you push new changes |
rustbot
added
S-blocked
|
Although I annotated my test with |
rustbot
added
the
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
Artur-Sulej
force-pushed
the
1775-decimal_bit_mask
branch
from
013adec to
12ab9f6
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
samueltardieu
added
S-waiting-on-author
| 99 | 0b1010; //~ decimal_bit_mask | ||
| 99 ^ 0b1010; //~ decimal_bit_mask | ||
| 0xD | 99; //~ decimal_bit_mask | ||
| 88 & 99; //~ decimal_bit_mask |
There was a problem hiding this comment.
Although I annotated my test with
//~ decimal_bit_mask, Clippy still reports the lint as an unmatched diagnostic and the test fails.
Any idea what I might be missing?
That's because in this case, you're firing the lint twice, once for each operand, so you'll need to add a second error mark. That's the easiest to do by bringing them to under the linted expression:
| 88 & 99; //~ decimal_bit_mask | |
| 88 & 99; | |
| //~^ decimal_bit_mask | |
| //~| decimal_bit_mask |
^means the lint should happen on the line above the comment|"attaches" the error mark to another one on a neighbouring line (in this case the//~^ decimal_bit_maskdirectly above), and will expect a lint at the same location (in this case, the line containing88 & 99;)
| Expr { | ||
| kind: kind1, | ||
| span: span1, | ||
| .. | ||
| }, | ||
| Expr { | ||
| kind: kind2, | ||
| span: span2, | ||
| .. | ||
| }, | ||
| ) = &e.kind |
There was a problem hiding this comment.
It would be a bit more concise to have these simply as left and right, and then use left.span, right.kind etc. throughout the code
| span_lint( | ||
| cx, | ||
| DECIMAL_BIT_MASK, | ||
| e.span, |
There was a problem hiding this comment.
You can improve the output by pointing the lint at the exact operand that is decimal -- for example, here you would use span1 (or left.span, as per previous comment)
| cx, | ||
| DECIMAL_BIT_MASK, | ||
| e.span, | ||
| "Using decimal literal for bit mask. Consider using binary (0b...) or hexadecimal (0x...) notation for better readability.", |
There was a problem hiding this comment.
The second sentence should be emitted separately, as a help message -- see https://doc.rust-lang.org/clippy/development/emitting_lints.html#emitting-a-lint-1 for more info.
Try using span_lint_and_help here
| && let Some(snippet) = snippet_opt(cx, *span2) | ||
| && !snippet.starts_with("0b") | ||
| && !snippet.starts_with("0x") |
There was a problem hiding this comment.
You can use SpanRangeExt::get_source_code here to avoid allocating a String (which is what snippet_opt does)
&& let Some(snippet) = span2.get_source_text(cx)In fact, since you're only using the snippet to check its contents, you can get away with SpanRangeExt::check_source_code:
| && let Some(snippet) = snippet_opt(cx, *span2) | |
| && !snippet.starts_with("0b") | |
| && !snippet.starts_with("0x") | |
| && span2.check_source_text(cx, |src| !src.starts_with("0b") && !src.starts_with("0x")) |
5 participants
changelog: [
decimal_bit_mask]: add a new lint that detects the use of decimal literals as bit masks in bitwise operations. Using decimal literals for bit masks can obscure the intended bit pattern and reduce code readability. This lint encourages the use of binary (0b...) or hexadecimal (0x...) notation to make bit patterns explicit and easier to understand at a glance.Addresses issue: #1775
Example:
Summary Notes
Managed by
@rustbot—see help for details